fix: Address modernize.omitzero issues
#3972
Conversation
alexandear
changed the title
modernize.omitzero issues
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3972 +/- ##
=======================================
Coverage 93.53% 93.53%
=======================================
Files 207 207
Lines 17586 17586
=======================================
Hits 16449 16449
Misses 937 937
Partials 200 200 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @alexandear!
LGTM.
As far as I can tell, though, this PR does NOT break the existing API, and therefore the PR title can change back to "fix:" instead of "fix!:". Do you agree?
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
alexandear
changed the title
modernize.omitzero issuesmodernize.omitzero issues
|
If you're updating the code for omitzero, the environment created update struct had annotations removed before omitzero was available. The omitzero annotation could now be added to reviewers and one of the other fields (can't remember the name off the top of my head). |
|
Thank you, @alexandear, @stevehipwell, and @Not-Dhananjay-Mishra! |
4 participants
This PR enables the previously disabled
modernize.omitzerocheck.Fixed issues
markReadOptions.LastReadAt:We can safely change
omitzerobecause it's an unexported struct. Now we can pass an emptyTimestamp{}toMarkNotificationsReadandMarkRepositoryNotificationsRead, which means marking all notifications as read.RepositoryContentResponse.Commit:It's a required field according to https://docs.github.com/en/rest/repos/contents?apiVersion=2022-11-28#create-or-update-file-contents.